Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(connector-stellar): add a deploy contract endpoint #3283

Conversation

fazzatti
Copy link
Contributor

@fazzatti fazzatti commented May 23, 2024

  • Add a Stellar Connector plugin following the same pattern as the Besu Connector plugin.
  • Add a deploy contract endpoint to the Stellar Connector plugin.

Initialization remarks:
Supports a network configuration object to define all integration services that seamlessly integrate with the Stellar test ledger within the Cacti test tooling.

Deploy remarks:
The deploy process supports both the compiled smart contract WASM as well as the on-chain WASM hash as inputs. This follows the smart contract deployment design on Soroban(Stellar's smart contract platform). Refer to the Stellar documentation for further detail on this process.

More details can be found in the README.md file under the connector root directory.

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

@fazzatti fazzatti force-pushed the feat-stellar-ledger-connector-deploy branch from 7aa73ec to c27a031 Compare May 23, 2024 14:26
@fazzatti fazzatti marked this pull request as ready for review May 23, 2024 14:27
@fazzatti fazzatti force-pushed the feat-stellar-ledger-connector-deploy branch from c27a031 to c583129 Compare May 23, 2024 20:01
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fazzatti Initial feedback before I do a detailed review:

  1. please fix the DCO
  2. include a CI job (ci.yaml) to execute the test(s)
  3. include all the information in the commit message not just the pull request description.

@fazzatti fazzatti force-pushed the feat-stellar-ledger-connector-deploy branch from c583129 to d35eab4 Compare May 27, 2024 15:06
@fazzatti
Copy link
Contributor Author

fazzatti commented May 27, 2024

@fazzatti Initial feedback before I do a detailed review:

  1. please fix the DCO
  2. include a CI job (ci.yaml) to execute the test(s)
  3. include all the information in the commit message not just the pull request description.

Thank you @petermetz !

  1. please fix the DCO

I think I've got it right now. I didn't notice it was required to have the written line in the commit directly.

  1. include a CI job (ci.yaml) to execute the test(s)

Just as opened the PR for review I thought 'perhaps the CI could've been changed in this change...'. Just did this now and I tried to follow the other examples here. Hope it is within the standard for the CI.

  1. include all the information in the commit message not just the pull request description.

Here I added additional bits when writing the PR and didn't think of going back to add those into the commit too but now I think I did update it correctly!

Thanks for the remarks! I appreciate!

@fazzatti fazzatti requested a review from petermetz May 27, 2024 16:46
@fazzatti fazzatti force-pushed the feat-stellar-ledger-connector-deploy branch from d35eab4 to 8b7d241 Compare May 28, 2024 17:28
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fazzatti You need to add the plugin-ledger-connector-stellar-changed filter to the outputs of the job as well, not just declare it as a filter. Otherwise it won't be accessible to the other job's if condition and will always evaluate to false ending up skipping the job.

The outputs are at the top of the job declaration and look like this (and the stellar one is missing)

  compute_changed_packages:
    outputs:
      cmd-api-server-changed: ${{ steps.changes.outputs.cmd-api-server-changed }}
      plugin-ledger-connector-polkadot-changed: ${{ steps.changes.outputs.plugin-ledger-connector-polkadot-changed }}
      plugin-ledger-connector-aries-changed: ${{ steps.changes.outputs.plugin-ledger-connector-aries-changed }}
      plugin-ledger-connector-besu-changed: ${{ steps.changes.outputs.plugin-ledger-connector-besu-changed }}
      plugin-ledger-connector-corda-changed: ${{ steps.changes.outputs.plugin-ledger-connector-corda-changed }}
      plugin-ledger-connector-fabric-changed: ${{ steps.changes.outputs.plugin-ledger-connector-fabric-changed }}
      plugin-ledger-connector-ethereum-changed: ${{ steps.changes.outputs.plugin-ledger-connector-ethereum-changed }}
      plugin-ledger-connector-iroha2-changed: ${{ steps.changes.outputs.plugin-ledger-connector-iroha2-changed }}
      plugin-ledger-connector-quorum-changed: ${{ steps.changes.outputs.plugin-ledger-connector-quorum-changed }}
      plugin-htlc-coordinator-besu-changed: ${{ steps.changes.outputs.plugin-htlc-coordinator-besu-changed }}
      test-tooling-changed: ${{ steps.changes.outputs.test-tooling-changed }}
      ghcr-corda-all-in-one-obligation-changed: ${{ steps.changes.outputs.ghcr-corda-all-in-one-obligation-changed }}
      ghcr-corda-all-in-one-changed: ${{ steps.changes.outputs.ghcr-corda-all-in-one-changed }}
      ghcr-dev-container-vscode-changed: ${{ steps.changes.outputs.ghcr-dev-container-vscode-changed }}

@fazzatti fazzatti force-pushed the feat-stellar-ledger-connector-deploy branch from 8b7d241 to 7c8e047 Compare June 3, 2024 13:19
@fazzatti
Copy link
Contributor Author

fazzatti commented Jun 3, 2024

@fazzatti You need to add the plugin-ledger-connector-stellar-changed filter to the outputs of the job as well, not just declare it as a filter. Otherwise it won't be accessible to the other job's if condition and will always evaluate to false ending up skipping the job.

The outputs are at the top of the job declaration and look like this (and the stellar one is missing)

  compute_changed_packages:
    outputs:
      cmd-api-server-changed: ${{ steps.changes.outputs.cmd-api-server-changed }}
      plugin-ledger-connector-polkadot-changed: ${{ steps.changes.outputs.plugin-ledger-connector-polkadot-changed }}
      plugin-ledger-connector-aries-changed: ${{ steps.changes.outputs.plugin-ledger-connector-aries-changed }}
      plugin-ledger-connector-besu-changed: ${{ steps.changes.outputs.plugin-ledger-connector-besu-changed }}
      plugin-ledger-connector-corda-changed: ${{ steps.changes.outputs.plugin-ledger-connector-corda-changed }}
      plugin-ledger-connector-fabric-changed: ${{ steps.changes.outputs.plugin-ledger-connector-fabric-changed }}
      plugin-ledger-connector-ethereum-changed: ${{ steps.changes.outputs.plugin-ledger-connector-ethereum-changed }}
      plugin-ledger-connector-iroha2-changed: ${{ steps.changes.outputs.plugin-ledger-connector-iroha2-changed }}
      plugin-ledger-connector-quorum-changed: ${{ steps.changes.outputs.plugin-ledger-connector-quorum-changed }}
      plugin-htlc-coordinator-besu-changed: ${{ steps.changes.outputs.plugin-htlc-coordinator-besu-changed }}
      test-tooling-changed: ${{ steps.changes.outputs.test-tooling-changed }}
      ghcr-corda-all-in-one-obligation-changed: ${{ steps.changes.outputs.ghcr-corda-all-in-one-obligation-changed }}
      ghcr-corda-all-in-one-changed: ${{ steps.changes.outputs.ghcr-corda-all-in-one-changed }}
      ghcr-dev-container-vscode-changed: ${{ steps.changes.outputs.ghcr-dev-container-vscode-changed }}

Thank you so much @petermetz !
It seems to be exactly it! I made the adjustments and as the CI is running right now, I can see the Cactus_CI / cpl-connector-stellar was executed and finished successfully!

@fazzatti fazzatti requested a review from petermetz June 3, 2024 13:46
@fazzatti
Copy link
Contributor Author

fazzatti commented Jun 3, 2024

@petermetz Just noticed that something else failed at the Cactus_CI / yarn_custom_checks this time. I'll have a look at it!

- Add a Stellar Connector plugin following the same pattern as the **Besu Connector plugin**.
- Add a deploy contract endpoint to the Stellar Connector plugin.

**Initialization remarks:**
Supports a network configuration object to define all integration services that seamlessly
integrate with the Stellar test ledger within the Cacti test tooling.

**Deploy remarks:**
The deploy process supports both the compiled smart contract WASM as well as the on-chain WASM
hash as inputs. This follows the smart contract deployment design on Soroban
(Stellar's smart contract platform). Refer to the Stellar documentation at:
https://developers.stellar.org/docs/learn/fundamentals/stellar-data-structures/contracts
for further detail on this process.

More details can be found in the `README.md` file under the connector root directory.

Signed-off-by: Fabricius Zatti <[email protected]>
@fazzatti fazzatti force-pushed the feat-stellar-ledger-connector-deploy branch from 7c8e047 to d29f769 Compare June 3, 2024 15:01
@fazzatti
Copy link
Contributor Author

fazzatti commented Jun 3, 2024

@petermetz Just noticed that something else failed at the Cactus_CI / yarn_custom_checks this time. I'll have a look at it!

I think it should be ok now!

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fazzatti You are most welcome, I'm glad we figured it out! Looking good now!

@petermetz petermetz merged commit e1d86c3 into hyperledger-cacti:main Jun 3, 2024
143 of 150 checks passed
@petermetz petermetz deleted the feat-stellar-ledger-connector-deploy branch June 3, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants